Fix division by zero in clDice loss harmonic mean#8739
Fix division by zero in clDice loss harmonic mean#8739Mr-Neutr0n wants to merge 2 commits intoProject-MONAI:devfrom
Conversation
…c mean The clDice loss computes the harmonic mean of topology precision (tprec) and sensitivity (tsens) as `2 * tprec * tsens / (tprec + tsens)`. When both values are zero -- which occurs with empty inputs, non-overlapping predictions, or when using smooth=0 -- this produces NaN due to 0/0 division. Add a small epsilon (1e-7) to the harmonic mean denominator to prevent division by zero. This has negligible effect on normal computation since tprec and tsens are bounded in [0, 1], but prevents NaN loss values that would otherwise crash training of tubular structure segmentation models. Also add test cases covering zero-input and non-overlapping edge cases with smooth=0 to verify the fix. Signed-off-by: Mr-Neutr0n <64578610+Mr-Neutr0n@users.noreply.github.com>
Mr-Neutr0n
requested review from
KumoLiu,
Nic-Ma and
ericspod
as code owners
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe pull request adds a small epsilon (1e-7) to the denominator of the cl_dice computation in two forward methods to avoid division-by-zero/near-zero, preserving the original formula. It also adds two unit tests verifying that SoftclDiceLoss and SoftDiceclDiceLoss do not produce NaN for zero-valued inputs and for non-overlapping predictions when smooth=0.0. Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hi @Mr-Neutr0n thanks for this change, there should be a value to prevent divide-by-zero but I would suggest following the pattern found with the other Dice losses. I would add a |
2 participants
Summary
SoftclDiceLossandSoftDiceclDiceLosswhen computing the harmonic mean of topology precision and sensitivity1e-7) to the denominator(tprec + tsens)to preventNaNwhen both values are zerosmooth=0Details
The clDice loss computes
cl_dice = 1.0 - 2.0 * (tprec * tsens) / (tprec + tsens). When bothtprecandtsensare zero (e.g., empty inputs, non-overlapping predictions/targets, orsmooth=0), this results in0/0 = NaN, which propagates through the loss and crashes training.While the default
smooth=1.0preventstprecandtsensfrom being exactly zero in most cases, settingsmooth=0(a valid configuration) exposes this bug whenever skeleton overlap is zero. The fix adds1e-7to the harmonic mean denominator, which:cl_dice = 1.0(maximum loss) when both precision and sensitivity are zero, which is the correct semantic resultsmooth_drinDiceLoss)Test plan
test_cldice_loss.pytests still pass (perfect overlap cases)test_zero_input_no_nan: verifies zero-valued inputs withsmooth=0do not produce NaNtest_no_overlap_no_nan: verifies non-overlapping predictions/targets withsmooth=0do not produce NaN